Implement the free and flag-gated latency audit findings - #1377
Conversation
The 2026-07-28 latency audit was proposed as PR #1312 and closed unmerged because its additive-index migration lacked synchronized schema/drift proof. The closing note also said "compatible latency work is already on main" — verification on 6021f6d found none of the six applied changes had landed, and the audit document itself was absent, so its follow-ups were untracked. This re-lands the free- and flag-gated work with tests, authors the operator SQL without touching supabase/**, and files the remainder in the ledger. Applied: - Server-Timing preamble stages (auth/ratelimit/scope). /api/answer/stream — the route the UI actually calls — emitted no header at all; /api/search emitted none either. Only pre-header stages can appear on the stream route, since routing in-stream durations through the SSE contract would put instrumentation inside a governed clinical payload. - L1-2: /api/answer resolves scope concurrently with the rate-limit RPC, aborts it on deny, and threads AbortSignal.any so a client disconnect cancels scope's paginated queries. The promise is settled, never floating. - L1-1: the shared-cache-hit path no longer awaits setCachedAnswer before responding. forceRefresh stays — it is the mid-request staleness guard, now documented on the function. - L2-6: three select("*") on document_table_facts narrowed to explicit projections, reusing tableFactDetailProjection. Keeps the generated search_tsv and owner_id off the wire; response shape unchanged. - L2-9: /api/medications builds the governance map and index projection once instead of per anonymous request. - L3-4/L3-5: ten ssr:false surfaces gain LoadingPanel fallbacks (role=status); Supabase preconnect/dns-prefetch added. Both ship ~zero bytes, so neither is gated behind #17, which governs payload decisions. Not applied, deliberately: the six canary-gated findings, the seven #17-gated findings, the four retired during verification, and the L2-3/L2-5 indexes — authored as reviewed CREATE INDEX CONCURRENTLY statements in the operator runbook with the apply/mirror/regenerate/register ordering spelled out. L4-2 is retracted: the worker's triple image read is a deliberate peak-memory trade-off already documented at worker/main.ts:866-869. RAG impact: no retrieval behaviour change — the only src/lib/rag/** edits defer a process-local cache write off the response path and add a doc comment; no scoring, ordering, selection, alias, or citation logic is touched, and the mid-request staleness guard is preserved. Verification: verify:cheap exit 0 (418 files, 4244 passed / 4 skipped); verify:pr-local exit 0 (same suite, production build compiled in 59s, client bundle secret surface check passed, 36 golden cases / 21 suites validated). tests/answer-route-preamble.test.ts fails against the pre-change serial shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change corrects latency-audit evidence and remediation documentation, adds module-level caching for anonymous medication payload derivations, and adds tests covering answer-route admission, abort propagation, error handling, and Server-Timing composition. ChangesLatency remediation and verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers. |
|
PR #1377 is cleaned up and mergeable again. Tip: Root causes
Changes made
Validation
Remaining blockerWait for hosted CI on |
|
@codex review PR |
|
@CodeRabbit Review PR |
|
✅ Action performedReview finished.
|
Resolve GitHub DIRTY/CONFLICTING staleness; merge-tree was clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b11fe98c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review raised this as P1 on PR #1377 and it is correct. The L1-2 change started resolveSearchScope concurrently with the rate-limit RPC and aborted it on deny, on the claim that a throttled caller "still costs nothing". That claim is false. resolveSearchScope only returns without touching the database when there are no filters and no explicit document ids (search-scope.ts:242,253). With either present it enters the paginated `documents` loop at :269 plus the nested label loop. An AbortSignal cancels the client request; it does not un-execute a statement Postgres has already begun. `filters` is caller-controlled, so a throttled caller could keep spending database capacity while collecting 429s — the opposite of what admission control is for, and the wrong direction against capacity-review.md:106-113, which names Postgres CPU under concurrency the first soft failure. Scope now sits behind admission again. Two parts of the original change are kept because they are independent of the overlap and unambiguously correct: - `signal: request.signal` is threaded into resolveSearchScope, so a client disconnect finally cancels its paginated queries. search-scope.ts:200,328 always supported .abortSignal(...); this route never passed one. - the `scope` stage is still reported in Server-Timing. tests/answer-route-preamble.test.ts is inverted to the guard the reviewer asked for: no scope query may begin before the limiter admits, and a denied request (sent with filters, the shape that reaches the paginated loop) dispatches none at all. Both cases fail against the overlapping shape. The audit's L1-2 section and ledger #99 record the refutation so a later latency pass does not rediscover the overlap; re-attempting it requires a non-database admission gate ahead of the durable limiter. Verification: verify:cheap exit 0 on the merged tree (423 files, 4278 passed / 4 skipped). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Keep the PR tip current after the CLAUDE.md orientation landing.
Capture the main sync, Codex P1 admission-order fix, and local verification on tip e353e1d.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e52c25c79a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Both raised at P2 by Codex review on PR #1377 and both verified against the code before acting. 1. The measurement plan still described L1-2's REFUTED contract — "scope must start before the limiter settles". The L1-2 section was corrected when the overlap was removed but this paragraph was missed, so the doc contradicted both the route and tests/answer-route-preamble.test.ts. Anyone generalising the harness under #98 would have enforced the opposite invariant and reintroduced database work for throttled requests. It now states the actual admission-cost contract: scope starts only after admission, and a denial dispatches zero scope queries. 2. The provider-free wall-clock step claimed the typeahead route plus demo mode times "auth + rate-limit + scope + RPC". False on both counts: src/app/api/search/universal/route.ts:137 returns on isDemoMode() || isLocalNoAuthMode() BEFORE createAdminClient (:150), publicAccessContext (:151) and consumeSubjectApiRateLimit (:153), and resolveSearchScope is never called from that route. A demo-mode run would measure local fixture search and appear to clear an L1 finding it never exercised. The step now says to run it non-demo, and to time scope separately via /api/answer. Docs only; no source or test behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Sync after #1378 landed; merge-tree was clean (GitHub DIRTY = staleness).
|
@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers. |
|
PR #1377 is MERGEABLE again at tip Root causes
Changes this pass
Validation
Remaining blockerWait for hosted CI on |
Sync after #1375 (Clinical Sky design tokens); merge-tree was clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b66d41d0a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apply step 5 and rollback phase A both described the required_indexes change as an edit to `search_schema_health()` at supabase/schema.sql:3177. schema.sql is a mirror, so that edit never reaches the hosted function. search_schema_health() is redefined by `create or replace function` in eleven migrations; 20260705180000_reconcile_search_health_indexes.sql is the precedent -- it creates indexes and carries the updated required_indexes array (:62) in the same migration. As written the procedure left the three new indexes unmonitored on live, and rollback was worse: phase A retracted only in the mirror, so phase B would drop indexes the hosted function still required and turn the health check red -- the exact failure phase A exists to prevent. Apply now authors one migration carrying both the index creates and the create-or-replace-function registration, mirrors both into schema.sql, and deploys last. Phase A gets its own retraction migration plus matching mirror and regenerated drift manifest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c11fdde972
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three places described the RAG-path index as canary-gated "until"/"unless"/ "or" fetchDocumentTitleAliasRows' unordered .limit(12) is made deterministic, which reads as ordering lifting the gate. It does not. An unordered LIMIT has no stable selection to preserve, so imposing an order can pick a different twelve than the database happens to return today -- that is an ordering behaviour change on a retrieval surface, which AGENTS.md already requires its own live eval-canary pair for. Sequencing the ordering fix first is still worthwhile, since an unordered LIMIT feeding retrieval candidates is latent nondeterminism regardless of this index, but it yields two canary-gated changes rather than one gate that ordering unlocks. The audit's L2-3 body already said this correctly; its opening clause and the two outstanding-issues rows and the runbook bullet did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ab16977d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
#103's outcome is that the migration chain and schema.sql agree on document_table_facts trigram indexes, but the row offered drift-allowlist.json as an alternative to mirroring. It is not one. The allowlist's own header scopes it to "Known live-vs-schema.sql divergence" -- it suppresses a live drift finding and cannot make the two schema sources agree. A fresh `supabase db reset` still runs 20260714190000 and creates document_table_facts_text_trgm_idx while schema.sql still omits it, so the divergence survives the allowlist entirely. There are exactly two routes: mirror it into schema.sql and regenerate the manifest if retained, or drop it via a forward migration if the live scan evidence shows it redundant. Also records that no offline gate catches this -- the migration/schema.sql parity test only asserts one migration's schema_drift_snapshot function definition, not an index inventory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Merge 9f7a629 brought origin/main into this branch, and merge=union kept both copies of four records this branch already carried from an earlier main merge. Static PR checks went red on the ledger guard: - 4 exact duplicate review record(s) found at line(s) 1277, 1279, 1280, 1281. - 4 record(s) repeat the same ref/HEAD/scope (line pairs 1270 and 1277; 1275 and 1279; 1272 and 1280; 1273 and 1281). This is the #88 watch condition exactly. All four pairs are byte-identical, so only the later copies are removed -- the one mutation the append-only contract permits. No record is edited and none is lost: 4 deletions, 0 additions, and the unique-row sets before and after are identical. The rows belong to three other branches (clinical-design-system-update-e34ca9, test-coverage-analysis-2vcd8a, document-reader-condensed-view), not to this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Resolved docs/outstanding-issues.md by keeping this branch's updated #98, #102, #104 and #105 entries (the ones this PR rewrites) and adding main's new #108 and #109 rows; the rest of the conflict was prettier re-padding column widths after a summary-cell length change. next-id stays at 110. Removed four exact-duplicate branch-review-ledger rows produced by the union merge driver keeping both copies of rows present on this branch and on main - the duplication risk tracked as #88.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d66138c4de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The #98 row still named test-cache-path.mjs and check-rag-fixtures.mjs as the fixtures to build the round-trip counting harness over. Neither can count a RAG request: the first computes Vitest/TypeScript cache paths, the second only validates fixture manifests. A harness wired to them would have pinned nothing. The audit document already carried this retraction (:358); this row did not, so the stale references survived in the one place an implementer would actually read them. Now names scripts/eval-rag-offline.mjs, scripts/test-rag-offline.mjs, scripts/rag-offline-contract.mjs and scripts/fixtures/rag-offline-contract-tests.json, all four verified present, and records the correction inline so it is not reintroduced a third time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #6558 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
| # | Test | Failure |
|---|---|---|
| 1, 2 | ui-phone-scroll.spec.ts:416 (browser + PWA) |
after dragScrollBy(page, 720, 24), universal-header-collapse never gains data-scroll-hidden="true" — 24 × locator resolved, attribute stays null for the full 10 s |
| 3 | ui-tools.spec.ts:945 /favourites?q=lithium at tablet |
getByTestId('search-query-ribbon') resolved to 2 elements, one under mobile-composer-reserve-pad |
Failure 3 matches open issue #093 exactly — "Next streaming S: clone causes Playwright strict-mode violations under CI load; a getByTestId that is unique locally resolves to 2 elements in CI."
Failures 1–2 are the interesting ones. The element resolves consistently, so it is not a duplicate or a missing-node problem; the scroll-hide simply never fires. The most likely reading is that the drag gesture produced no actual scroll movement under load, so the listener never ran — but I want to be careful not to over-claim: on the evidence available these are new tests that have run green exactly once and red exactly once, which is not enough to call either way.
What I am not doing, and why
- Not quarantining.
docs/testing.md:59requires reproducing three times on the same SHA, an owner, a repro command and an expiry, and says explicitly to "treat repeatable failures as regressions". A one-off red does not qualify, and the flake ledger governs these identities. - Not fixing the phone-chrome behaviour. It is
main's code, outside this PR's scope, anddocs/search-chrome-behaviour.mdgoverns that surface — changing it needs that document read first andverify:phone-chromerun, not an opportunistic patch from a latency PR. - Not re-running the job unilaterally. Hosted CI is confirmation-required here, and the triage bot's own instruction is to rerun only after classifying the cause. The cause is now classified; a re-run is the natural next step and I will do it on request.
The substantive point for the repo, independent of this PR: #1389 landed a phone-chrome change whose tests have been executed exactly twice, and main has not run them since the merge because every push to main afterwards was docs-only. If these are flaky, they will keep reddening unrelated PRs; if they are real, main is currently broken on the document-detail phone surface. Either way it wants three same-SHA reproductions to settle, which is a decision for the repo owner rather than something to resolve from inside this PR.
Generated by Claude Code
Production UI red on
|
Resolves the `docs/outstanding-issues.md` conflict. Two independent causes: 1. Rows #98–#105 diverged. `main` carries the newer text (PR #1377 landed "Done 2026-07-29" outcomes and rewrote several summaries); this branch carried the pre-#1377 wording. Took `main`'s rows wholesale — the branch made no edits to that range, it simply forked before them. 2. `#108`/`#109` were allocated twice. `main` used them for the verified-landed branch deletions and the shallow-clone finding; this branch had used the same two ids for the design-system token manifest and the `ui-overlap` flake. The `issues:next-id` marker has no concurrency protection, which is exactly the failure the branch's own PR notes predicted. Renumbered this branch's two rows to #110/#111 and bumped the marker to 112. No content was dropped from either side; all 111 rows are unique. Also records #111 as done, since this branch is what fixes it: the ui-overlap inset measurement now retries inside `toPass` with the 2px symmetry tolerance and the assertions unchanged. Leaving it open with a "Next: apply the retry shape" action would have re-queued work this PR already did.


Summary
6021f6dfound none of the six applied changes had landed, and the audit document itself was absent, so its follow-ups were untracked anywhere.Server-Timingpreamble stages (auth/ratelimit/scope). This was the repo's largest measurement gap:/api/answer/stream— the route the UI actually calls — emitted no header at all, and/api/searchemitted none either. Only pre-header stages appear on the stream route, because headers flush before the first SSE frame and routing in-stream durations through the SSE contract would put instrumentation inside a governed clinical payload (answer-stream-contract.ts:18-21whitelists onlyprogress/final/error)./api/answerthreadssignal: request.signalintoresolveSearchScope, fixing a pre-existing defect where a client disconnect could not cancel scope's paginated queries even thoughsearch-scope.ts:200,328always supported.abortSignal(...). The concurrency part of this finding was written, reviewed, and removed — see "Correction" below. Scope resolution runs behind rate-limit admission.setCachedAnswerbefore responding, so serving a cached answer no longer requires a freshdocumentsquery first.forceRefresh: trueis deliberately kept: it is the mid-request staleness guard that discards the write when the corpus moves, and that constraint is now documented on the function itself.select("*")calls ondocument_table_factsnarrowed to explicit projections, reusing the existingtableFactDetailProjectionrather than duplicating it. This keeps the generatedsearch_tsvtsvector andowner_idoff the wire. The PATCH response shape is unchanged — the projection matches theTableFactRowDTO field for field./api/medicationsbuilds the governance map and thefields=indexprojection once at module scope instead of mapping every record on every anonymous request. Ranking still runs per query.ssr: falsedashboard surfaces rendered nothing between HTML arrival and chunk execution and now use the sharedLoadingPanelprimitive (role="status"plus an accessible label); the Supabase origin gainspreconnectanddns-prefetch, sinceAuthProviderawaits a cross-origingetUser()on mount that every auth-gated fetch queues behind. Neither is held behind ledger#017, which governs payload decisions — a loading fallback ships zero bytes and a resource hint ships about sixty.#017-gated findings (need live Web-Vitals evidence first), and the four the audit retired during verification. All are filed as ledger#098–#105.supabase/**change: the threeCREATE INDEX CONCURRENTLYstatements live indocs/operator-apply-performance-latency-remediation.mdwith the apply → mirror → regenerate → register ordering spelled out. Registering the index names inrequired_indexesfirst would turn the live health check red, and shipping the migration without the schema mirror is precisely what closed PR chore: organize dirty work from claude/latency-audit-f1cbcd #1312.worker/main.ts:866-869and dispositioned as such by the 2026-07-01 audit. Recorded as a correction in#104so a third audit does not resurrect it.RAG impact: no retrieval behaviour change — the only
src/lib/rag/**edits defer a process-local cache write off the response path and add a doc comment; no scoring, ordering, selection, alias, or citation logic is touched, and the mid-request staleness guard is preserved.Correction — L1-2's concurrency was refuted on review (
e52c25c)An earlier revision of this PR started
resolveSearchScopeconcurrently with the rate-limit RPC and aborted it on deny, and this description claimed a throttled caller "still costs nothing". That claim was wrong, as Codex review raised at P1 and I confirmed against the code.resolveSearchScopereturns without touching the database only when there are no filters and no explicit document ids (the early returns atsearch-scope.ts:242and:253). With either present it enters the paginateddocumentsloop at:269plus the nested label loop — and anAbortSignalcancels the client request without un-executing a statement Postgres has already begun. Becausefiltersis caller-controlled, a throttled caller could keep spending database capacity while collecting 429s. That is the opposite of what admission control is for, and the wrong direction againstcapacity-review.md:106-113, which names Postgres CPU under concurrency the first soft failure — the very argument this PR uses for why round-trip reduction has capacity value.Scope now sits behind admission. What is kept from the original change is the threaded abort signal and the
scopetiming stage, both of which are independent of the overlap and correct on their own.tests/answer-route-preamble.test.tsis the guard the reviewer asked for: no scope call may begin while the limiter is pending, and a denied request sent with filters must dispatch none at all. Both cases were checked to fail against the overlapping shape rather than pass vacuously. The refutation and its precondition — a non-database admission gate ahead of the durable limiter — are recorded in the audit's L1-2 section and ledger#099so a later latency pass does not rediscover it as an obvious win.Verification
npm run verify:pr-local— exit 0. Format, lint, typecheck,Test Files 418 passed (418)/Tests 4244 passed | 4 skipped (4248), production build✓ Compiled successfully in 59s,Client bundle secret surface check passed.,Offline RAG fixture and manifest validation passed (36 golden cases, 21 suites).npm run verify:cheap— exit 0 after themainmerge and the L1-2 correction:Test Files 423 passed (423)/Tests 4278 passed | 4 skipped (4282).npm run check:branch-review-ledger— passed after themainmerge, guarding themerge=unionduplication risk that ledger#088watches for.UI verification not run: the UI change is ten
loadingfallbacks built from the existing sharedLoadingPanelprimitive plus two<link>resource hints, with no new markup, styling, layout, or motion. Per the agreed scope for this pass,npm run verify:uiis deferred and tracked in ledger#105, which also asks for confirmation that the preconnect reaches<head>on a live page.npm run eval:retrieval:quality,eval:rag,eval:quality,check:supabase-projectandverify:releasewere not run: all are provider-backed, and no retrieval, ranking, selection, chunking, scoring, or answer-generation behaviour changed.check:driftanddrift:manifestwere not run becausesupabase/**is untouched, so there is nothing to re-derive.Note on the red
PR requiredruns: the job log showsCOVERAGE_RESULT: success,BUILD_RESULT: success,STATIC_RESULT: success,SAFETY_RESULT: success,DB_RESULT: success, andUI_RESULT: cancelled→##[error]production-ui result was cancelled. That is ledger#095— the aggregate callsrequire_successonproduction-ui, so a push that supersedes an in-flight run reports failure indistinguishably from a real one. No substantive job failed.Risk and rollout
/api/answerpreamble is fully sequential, so this PR adds no concurrency to the answer path and no work to denied requests; the ordering is pinned by a test that fails against the overlapping shape. The cache-write deferral cannot change what is served — it moves a write that was already discarded on a staleness mismatch off the response path, and the discard condition is untouched. The narrowed projections were checked field-by-field against the consuming DTO. The medication memo caches only query-independent derivations of an already-memoised snapshot.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes on the above: no answer content, citation, verification, or governance logic is touched, so source-backed claims and conservative source-metadata behaviour are unchanged by construction. Narrowing the table-facts projections strictly reduces what leaves the server — it stops
owner_idbeing returned on the PATCH response. Thepreconnectreads only the already-publicNEXT_PUBLIC_SUPABASE_URLand emits nothing when that variable is absent, so demo mode is unaffected and no credential enters the client graph; the client bundle secret surface check passed. The Supabase target is unchanged because no Supabase configuration is touched at all. There is no change to clinical decision-support behaviour, so the SaMD classification is unaffected.Notes
#098–#105are new and replace the#085–#092numbering the original audit draft used; those IDs were taken by unrelated items before this landed.#016gains the L3-1/L3-2/L3-3/L3-6/L3-7 detail and a correction to its Therapy Compass framing: those files are unversioned and served with an ETag, so repeat visits pay revalidation round trips rather than the full 3.16 MB, and the fix is content-hashed filenames rather than a bareCache-Controlline.#017exemption for L3-4/L3-5 is a deliberate reading, stated in both the audit and#105:#017gates payload decisions, and a zero-byte fallback cannot be justified or refuted by a Lighthouse number.🤖 Generated with Claude Code
https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Summary by CodeRabbit
/api/answerrequest admission, abort propagation, and error handling to avoid unnecessary downstream work.ssr:falserendering paths to consistently show an accessible loading fallback.